Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connectComponent, attaching prop configuration to component #185

Closed
wants to merge 8 commits into from

Conversation

tgriesser
Copy link
Contributor

Alternate approach to #179 for consideration, adds a connectFactory connectComponent which connect will wrap, allowing for unique selectors per-component instance. Existing use of connect will work the same as it does currently.

* master:
  rename smart/dumb to presentational/container
  Add jsnext:main
  Remove src from npmignore
@tgriesser tgriesser changed the title connectFactory, attaching prop configuration to component connectComponent, attaching prop configuration to component Nov 25, 2015
@gaearon
Copy link
Contributor

gaearon commented Jan 27, 2016

What if instead we checked the return type of mapStateTo* functions when they are invoked for the first time. If the return type is a function rather than an object, assume we are dealing with a factory. This would be backwards compatible and wouldn't require a new top-level export. Am I missing something?

@ellbee
Copy link
Contributor

ellbee commented Jan 27, 2016

Oh, good shout. I don't think you are missing anything. I just hacked together a quick test for mapStateToProps, and it seems like it would be a reasonable solution.

@gaearon
Copy link
Contributor

gaearon commented Feb 2, 2016

@tgriesser Would you be interested in implementing #185 (comment) as an alternative approach? Would it solve your problem?

@gaearon
Copy link
Contributor

gaearon commented Feb 2, 2016

I’ll close this PR because I don’t plan to merge it as is but I’m happy to consider merging #185 (comment) which should solve the same problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants